Skip to content

Conversation

colesbury
Copy link
Contributor

Description

Fixes potential thread-safety issues if types are concurrently registered while get_local_type_info() is called in free threaded Python.

Use the internals mutex to also protect local_internals. This keeps the locking strategy simpler, and we already follow this pattern in some places, such as pybind11_meta_dealloc.

Fixes #5799

Suggested changelog entry:

  • Fix thread-safety issues if types are concurrently registered while get_local_type_info() is called in free threaded Python.

Fixes potential thread-safety issues if types are concurrently
registered while `get_local_type_info()` is called in free threaded
Python.

Use the `internals` mutex to also protect `local_internals`. This
keeps the locking strategy simpler, and we already follow this pattern
in some places, such as `pybind11_meta_dealloc`.
@rwgk
Copy link
Collaborator

rwgk commented Oct 6, 2025

@swolchok @oremanj any chance one of you could help out reviewing this PR?

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly over my head TBH, but I think I understand this enough to be comfortable merging it.

Mostly just curious and to see if I understand correctly:

I find the foo_with_lock_held(); foo() { LOCK; foo_with_lock_held(); } pattern more readable than the with_internals pattern. Could we in theory replace all with_internals with the lock_held approach?

@rwgk rwgk merged commit 9f75202 into pybind:master Oct 12, 2025
85 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Oct 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: local_internals appear very thread-unsafe

2 participants